Skip to content

Enabled no-console eslint rule + error instead of warning#585

Merged
benesjan merged 7 commits into
masterfrom
janb/no-console-log
May 17, 2023
Merged

Enabled no-console eslint rule + error instead of warning#585
benesjan merged 7 commits into
masterfrom
janb/no-console-log

Conversation

@benesjan

@benesjan benesjan commented May 16, 2023

Copy link
Copy Markdown
Contributor

Description

Fixes #446 + more strict eslint rules

Checklist:

  • I have reviewed my diff in github, line by line.
  • Every change is related to the PR description.
  • I have linked this pull request to the issue(s) that it resolves.
  • There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
  • The branch has been merged or rebased against the head of its merge target.
  • I'm happy for the PR to be merged at the reviewer's next convenience.

@benesjan benesjan marked this pull request as draft May 16, 2023 12:49
@spalladino

Copy link
Copy Markdown
Contributor

Don't we have legitimate uses of console.log in the code that need to be exempted by an eslint-disable...?

@benesjan

Copy link
Copy Markdown
Contributor Author

Don't we have legitimate uses of console.log in the code that need to be exempted by an eslint-disable...?

Yes. It's just that I like breaking tests and then fixing them in separate commits.

@spalladino

Copy link
Copy Markdown
Contributor

Sorry, I guess I jumped the gun once I saw the PR. I'll wait until it's out of draft!

@benesjan benesjan force-pushed the janb/no-console-log branch from e1598c5 to 0f7f491 Compare May 17, 2023 08:03
@benesjan benesjan changed the title Enabled no-console eslint rule Enabled no-console eslint rule + error instead of warning May 17, 2023
@benesjan benesjan marked this pull request as ready for review May 17, 2023 08:40
@benesjan benesjan requested a review from spypsy May 17, 2023 08:40
const end1 = new Date().getTime() - start1;
// const end1 = new Date().getTime() - start1;

console.log(`Single hasher: ~${end1 / values.length}ms / value`);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left comments

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, I think here it's ok to leave the console.log statements commented out because it's just for when people run it locally + the whole test is a messy WIP.

@benesjan benesjan merged commit c1fa0a1 into master May 17, 2023
@benesjan benesjan deleted the janb/no-console-log branch May 17, 2023 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add no-console rule to eslint

3 participants